Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Simplified API to import from FFI #854

Merged
merged 1 commit into from
Feb 19, 2022
Merged

Simplified API to import from FFI #854

merged 1 commit into from
Feb 19, 2022

Conversation

jorgecarleitao
Copy link
Owner

Inspired by this observation from @multimeric, I just realized that we do not really need a Field to read an array - the DataType is sufficient.

This PR leverages this observation to simplify the API a bit further. The updated examples show the idea.

@codecov
Copy link

codecov bot commented Feb 18, 2022

Codecov Report

Merging #854 (a8be8a2) into main (6ccb154) will increase coverage by 0.00%.
The diff coverage is 75.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #854   +/-   ##
=======================================
  Coverage   71.49%   71.50%           
=======================================
  Files         335      335           
  Lines       17910    17906    -4     
=======================================
- Hits        12805    12803    -2     
+ Misses       5105     5103    -2     
Impacted Files Coverage Δ
src/array/boolean/ffi.rs 0.00% <0.00%> (ø)
src/array/fixed_size_binary/ffi.rs 0.00% <0.00%> (ø)
src/array/map/ffi.rs 0.00% <0.00%> (ø)
src/array/null.rs 38.23% <0.00%> (ø)
src/array/union/ffi.rs 0.00% <0.00%> (ø)
src/ffi/schema.rs 68.47% <62.50%> (ø)
src/array/binary/ffi.rs 60.71% <100.00%> (ø)
src/array/fixed_size_list/ffi.rs 86.66% <100.00%> (ø)
src/array/list/ffi.rs 65.51% <100.00%> (ø)
src/array/primitive/ffi.rs 58.33% <100.00%> (ø)
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6ccb154...a8be8a2. Read the comment docs.

@multimeric
Copy link

Thanks for looking into this. I don't quite understand how this improves the API though. I guess we no longer have to import the schema using the FFI? But how can we pass the data type between arrow implementations in a portable way? Is it just an int enum and therefore this isn't an issue?

Also I don't understand how you can pass the data type into the C function that expects a schema pointer. How does that work at all? Are we not losing information stored in the Schema as well?

My comment was mostly about wanting a RecordBatch struct that bundled the Schema with the Arrays. Is the idea with this PR that the Schema isn't needed, and so a Chunk is therefore equivalent to a RecordBatch?

@jorgecarleitao
Copy link
Owner Author

Is the idea with this PR that the Schema isn't needed, and so a Chunk is therefore equivalent to a RecordBatch?

Yes, that is the core observation - exporting an Array does not require a schema nor fields, only its datatype. Because every array stores such a datatype in this implementation of Arrow, we can just expose the API for that.

This does not close #854 , they are related but independent :)

@jorgecarleitao
Copy link
Owner Author

Also I don't understand how you can pass the data type into the C function that expects a schema pointer. How does that work at all? Are we not losing information stored in the Schema as well?

What is "the C function"? What is "Schema"?

There are two things supported by the C data interface:

  1. A Field (Ffi_ArrowSchema)
  2. An Array (Ffi_ArrowArray)

@jorgecarleitao jorgecarleitao merged commit 4d9e1e3 into main Feb 19, 2022
@jorgecarleitao jorgecarleitao deleted the simpler_ffi branch February 19, 2022 13:13
sydduckworth pushed a commit to mindx/arrow2 that referenced this pull request Mar 2, 2022
sydduckworth pushed a commit to mindx/arrow2 that referenced this pull request Mar 2, 2022
@jorgecarleitao jorgecarleitao changed the title Simplified API for FFI Simplified API to import from FFI Mar 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants